-
Notifications
You must be signed in to change notification settings - Fork 13.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add latest tag action #11148
chore: add latest tag action #11148
Conversation
36d82b2
to
dba1cd6
Compare
dba1cd6
to
8dcf2da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool automation! maybe drop a comment that this will automatically run on https://github.com/apache/incubator-superset/blob/master/RELEASING/README.md and a different one superset docs/instalation
cc1011e
to
b87b162
Compare
good idea. I had an earlier PR with docs/instructions to check out the latest before this tag that was blocked, so I just included it in here. |
Codecov Report
@@ Coverage Diff @@
## master #11148 +/- ##
===========================================
- Coverage 67.68% 54.89% -12.80%
===========================================
Files 930 421 -509
Lines 45132 14810 -30322
Branches 4331 3821 -510
===========================================
- Hits 30549 8130 -22419
+ Misses 14480 6680 -7800
+ Partials 103 0 -103
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. My only worry is the scenario where we might do a fix to an old version (say 0.37.3
) after we've released a more recent major/minor 0.38.0
. In that case I believe this would point latest
to 0.37.3
. I think this won't be a problem right now, but once we hit 1.0
, we'll probably start having this problem when we start releasing new majors while maintaining minors for stability.
8d8d5e0
to
0f1ab3f
Compare
That's a good point. Let me see if there's a check I can do in this action to make this more accurate. |
80ee970
to
1bd4b69
Compare
7101943
to
ceb8e43
Compare
@villebro I updated the logic in the action to see if the new release is tagged with a later version, if you want to take another look. thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments. I wonder if this could be changed into a bash script that could also be run from the command for testing? There's lots of really helpful echos here that could be helpful when run locally when further developing this functionality.
- name: Check for latest tag | ||
id: latest-tag | ||
run: | | ||
LATEST_TAG=$(git show-ref --tags -d | grep latest^{} | sed 's/refs\/tags\/latest^{}//') || echo 'not found' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It took me some time to figure out what ^{}
means in this context (I thought it was some non-standard regex in grep 😆 ). Perhaps we could add a comment here explaining what we're greping and seding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wound up changing the lookup on this, so that we didn't have to look for the tag with the pointer sha. It should read a bit better now.
LATEST_RC_TAG_SPLIT=(${LATEST_RC_TAG}) | ||
|
||
## remove the 'rc' from the current tag if it exists and split into an array at the dot | ||
THIS_TAG=($(echo ${{ github.event.release.tag_name }} | sed 's/rc//')) || echo 'not found' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably don't want to consider release candidates for latest
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks.. I removed them.
4d423dc
to
b6a7ef1
Compare
b6a7ef1
to
589c4ef
Compare
@villebro this should be ready for another look. I pulled the script out of the github action, so it can be run separate, with or without a --dry-run flag. You can also use a tool like |
Found one more bug I want to test... |
8a0dbb5
to
db2b28c
Compare
I tested a few more times, and it's looking stable so far. Maybe we should consider some bash automated testing systems? |
description: Superset latest release | ||
tag-name: latest | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ktmud may have some insight on using secrets.GITHUB_TOKEN
vs github.GITHUB_TOKEN
. I haven't seen the latter in documentation, although it seems that using it over the secrets
could be worth looking at.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems there is no github.GITHUB_TOKEN
available according to the doc, but github.token
is equivalent to secrets.GITHUB_TOKEN
: https://docs.github.com/en/free-pro-team@latest/actions/reference/context-and-expression-syntax-for-github-actions#github-context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great. thanks for that. That makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@villebro I caught another small bug during demo. I'll put up a fix today. |
Ok, just one more small change ba31898 It shouldn't affect the rest of the functionality, it's just one more check to be sure that the tag passed in exists. It won't be an issue when using the action itself, but could happen when someone uses the script directly. |
When we land this, we should create a latest tag on 0.38.0. |
* upstream/master: (55 commits) feat(explore): time picker enhancement (apache#11418) feat: update alert/report icons and column order (apache#12081) feat(explore): metrics and filters controls redesign (apache#12095) feat(alerts/reports): add refresh action (apache#12071) chore: add latest tag action (apache#11148) fix(reports): increase crontab size and alert fixes (apache#12056) Small typo fix in Athena connection docs (apache#12099) feat(queries): security perm simplification (apache#12072) feat(databases): security perm simplification (apache#12036) feat(dashboards): security permissions simplification (apache#12012) feat(logs): security permissions simplification (apache#12061) chore: Remove unused CodeModal (apache#11972) Fix typescript error (apache#12074) fix: handle context-dependent feature flags in CLI (apache#12088) fix: Fix "View in SQLLab" bug (apache#12086) feat(alert/report): add 'not null' condition option to modal (apache#12077) bumping superset ui to 15.18 and deckgl to 0.3.2 (apache#12078) fix: Python dependencies in apache#11499 (apache#12079) reset active tab on open (apache#12048) fix: improve import flow UI/UX (apache#12070) ...
SUMMARY
Action that adds a "latest" tag to release. It also overwrites/removes the last "latest" tag.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION